Skip to content

fix: har-10158 - fix comments highlighting and identify#707

Merged
harbournick merged 2 commits intodevelopfrom
comments_plugin
Jul 29, 2025
Merged

fix: har-10158 - fix comments highlighting and identify#707
harbournick merged 2 commits intodevelopfrom
comments_plugin

Conversation

@falsaadehh
Copy link
Copy Markdown
Contributor

No description provided.

@falsaadehh falsaadehh marked this pull request as draft July 27, 2025 21:50
@falsaadehh falsaadehh added the don't merge Don't merge yet label Jul 27, 2025
@falsaadehh falsaadehh marked this pull request as ready for review July 28, 2025 12:48
@falsaadehh falsaadehh requested review from VladaHarbour, artem-harbour and harbournick and removed request for harbournick July 28, 2025 12:48
@falsaadehh falsaadehh changed the title Comments plugin fix: har-10158 - fix comments highlighting and identify Jul 28, 2025
@linear
Copy link
Copy Markdown

linear Bot commented Jul 28, 2025

let activeThreadId = commentId;
tr.setMeta(CommentsPluginKey, { type: 'setActiveComment', activeThreadId });
tr.setMeta(CommentsPluginKey, { type: 'setActiveComment', activeThreadId: commentId, forceUpdate: true });
dispatch(tr);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In custom commands, no need to dispatch a transaction, it happens automatically.

const { from } = findRangeById(state.doc, id) || {};
if (from != null) {
const tr = state.tr.setSelection(TextSelection.create(state.doc, from));
dispatch(tr);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In custom commands, no need to dispatch a transaction, it happens automatically.

element?.scrollIntoView({ behavior: 'smooth', block: 'center' });
const element = document.querySelector(`[data-id="${newVal}"]`);
if (element) {
element?.scrollIntoView({ behavior: 'smooth', block: 'center' });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you have if check for element, what is the meaning of element?.

const editor = proxy.$superdoc.activeEditor;
activeComment.value = props.comment.commentId;
props.comment.setActive(proxy.$superdoc);
if (editor?.commands?.setCursorById) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to check the editor instance only, not the presence of the command.

Comment thread packages/superdoc/src/stores/comments-store.js Outdated
Comment thread packages/super-editor/src/extensions/comment/comments-plugin.js
Copy link
Copy Markdown
Contributor

@artem-harbour artem-harbour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added minor comments to fix.

@harbournick - please review comments functionality.

Copy link
Copy Markdown
Collaborator

@harbournick harbournick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread packages/super-editor/src/extensions/comment/comments-plugin.js
@harbournick harbournick merged commit b3ba884 into develop Jul 29, 2025
14 of 20 checks passed
@harbournick harbournick deleted the comments_plugin branch July 29, 2025 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

don't merge Don't merge yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants